Move attack troop overlay to WebGL#3996
Merged
Merged
Conversation
Contributor
WalkthroughMigrates attacking-troop labels from a DOM overlay to WebGL. Adds AttackingTroopsController that polls clustered attack positions, interpolates Slot animations, and emits per-frame AttackTroopLabel arrays to WorldTextPass; GameView/Renderer/WorldTextPass are extended to accept and render these labels. ChangesAttack Troop Labels Migration to WebGL
Sequence Diagram(s)sequenceDiagram
participant RAF as RequestAnimationFrame
participant Controller as AttackingTroopsController
participant Worker as myPlayer
participant WebGL as WebGLGameView
participant Pass as WorldTextPass
Controller->>Controller: tick() every 200ms
Controller->>Worker: attackClusteredPositions()
Worker-->>Controller: ClusteredCell[] response
Controller->>Controller: reconcileSlots() interpolate
Note over Controller: Slot-based smooth animation
RAF->>Controller: pushLabels() every frame
Controller->>WebGL: setAttackTroopLabels(AttackTroopLabel[])
WebGL->>Pass: setAttackTroopLabels()
Note over Pass: Generate GPU instances with zoom-scaled size
Pass-->>RAF: Render to screen
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly Related PRs
Suggested Labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/client/render/gl/passes/WorldTextPass.ts`:
- Around line 47-54: Move the hardcoded attack-label tuning values
(ATTACK_LABEL_SCREEN_SCALE, ATTACK_LABEL_OUTLINE_WIDTH and the other constants
used in WorldTextPass methods) into RenderSettings and read them from the
existing RenderSettings instance instead of using literals in WorldTextPass; add
corresponding keys and sane defaults to render-settings.json and the
RenderSettings type/interface, then replace uses of ATTACK_LABEL_SCREEN_SCALE,
ATTACK_LABEL_OUTLINE_WIDTH and the other pass constants inside the WorldTextPass
class (and any methods referencing them) with lookups like
renderSettings.attackLabelScreenScale / attackLabelOutlineWidth so the pass uses
configuration-driven values.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a4d82013-e76d-4597-a84d-383c95f34691
⛔ Files ignored due to path filters (1)
src/client/render/gl/shaders/world-text/world-text.frag.glslis excluded by!**/*.glsl
📒 Files selected for processing (7)
src/client/controllers/AttackingTroopsController.tssrc/client/hud/GameRenderer.tssrc/client/hud/layers/AttackingTroopsOverlay.tssrc/client/render/gl/GameView.tssrc/client/render/gl/Renderer.tssrc/client/render/gl/passes/WorldTextPass.tstests/client/graphics/layers/AttackingTroopsOverlay.test.ts
💤 Files with no reviewable changes (1)
- src/client/hud/layers/AttackingTroopsOverlay.ts
github-project-automation
Bot
moved this from Triage
to Development
in OpenFront Release Management
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/client/controllers/AttackingTroopsController.ts (1)
236-259: ⚡ Quick winReuse
labelBufinstead of allocating a new array every frame.This method runs on every animation frame (60+ fps). Creating
const out: AttackTroopLabel[] = []each time adds GC pressure. The class already has alabelBuffield meant for reuse—clear it and push into it directly.♻️ Proposed fix
private pushLabels(): void { if (this.alternateView || this.attacks.size === 0) { if (this.labelBuf.length > 0) { - this.labelBuf = []; + this.labelBuf.length = 0; this.view.setAttackTroopLabels(this.labelBuf); } return; } const now = performance.now(); - const out: AttackTroopLabel[] = []; + this.labelBuf.length = 0; for (const entry of this.attacks.values()) { const r = entry.isIncoming ? INCOMING_R : OUTGOING_R; const g = entry.isIncoming ? INCOMING_G : OUTGOING_G; const b = entry.isIncoming ? INCOMING_B : OUTGOING_B; for (const slot of entry.slots) { const t = Math.min(1, (now - slot.startMs) / ANIM_MS); slot.curX = slot.srcX + (slot.dstX - slot.srcX) * t; slot.curY = slot.srcY + (slot.dstY - slot.srcY) * t; - out.push({ + this.labelBuf.push({ x: slot.curX, y: slot.curY, text: entry.text, colorR: r, colorG: g, colorB: b, }); } } - this.labelBuf = out; - this.view.setAttackTroopLabels(out); + this.view.setAttackTroopLabels(this.labelBuf); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/client/controllers/AttackingTroopsController.ts` around lines 236 - 259, The loop currently allocates a fresh array with "const out: AttackTroopLabel[] = []" every frame; instead reuse the existing this.labelBuf: clear it (e.g., this.labelBuf.length = 0) before the loops and push each computed AttackTroopLabel into this.labelBuf while computing positions with performance.now(), ANIM_MS, and this.attacks; remove the local out and after the loops call this.view.setAttackTroopLabels(this.labelBuf) so no new array is created each frame.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/client/controllers/AttackingTroopsController.ts`:
- Around line 81-91: The init() method starts a perpetual requestAnimationFrame
loop (drive) and registers an AlternateViewEvent listener but never cleans them
up; add a dispose() method that (1) removes the AlternateViewEvent listener
registered via this.eventBus.on(...) (use the same handler reference or store it
as e.g. this._onAlternateView) and (2) cancels the RAF loop by storing the ID
returned from requestAnimationFrame when starting drive (e.g. this._rafId) and
calling cancelAnimationFrame(this._rafId) in dispose(); ensure dispose() also
nulls any references used by pushLabels()/drive to allow GC.
- Around line 161-162: AttackingTroopsController currently calls
renderTroops(troops) which delegates to renderNumber and produces hardcoded
"K"/"M" suffixes; change renderNumber (and/or renderTroops) to stop emitting
hardcoded suffixes and instead call translateText() with new keys (e.g.
"number.k", "number.m" or "troops.format") defined in resources/lang/en.json so
localization can control "K"/"M" and any surrounding formatting; update
renderTroops/renderNumber to: compute scaled value and pass the scale key to
translateText along with the formatted numeric part, and add the corresponding
keys and example values to resources/lang/en.json; ensure
AttackingTroopsController.ensureEntry still uses renderTroops(…) unchanged from
the caller perspective.
---
Nitpick comments:
In `@src/client/controllers/AttackingTroopsController.ts`:
- Around line 236-259: The loop currently allocates a fresh array with "const
out: AttackTroopLabel[] = []" every frame; instead reuse the existing
this.labelBuf: clear it (e.g., this.labelBuf.length = 0) before the loops and
push each computed AttackTroopLabel into this.labelBuf while computing positions
with performance.now(), ANIM_MS, and this.attacks; remove the local out and
after the loops call this.view.setAttackTroopLabels(this.labelBuf) so no new
array is created each frame.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a384bd5c-4c72-4649-9c30-78fe7ffdb511
⛔ Files ignored due to path filters (1)
src/client/render/gl/shaders/world-text/world-text.frag.glslis excluded by!**/*.glsl
📒 Files selected for processing (7)
src/client/controllers/AttackingTroopsController.tssrc/client/hud/GameRenderer.tssrc/client/hud/layers/AttackingTroopsOverlay.tssrc/client/render/gl/GameView.tssrc/client/render/gl/Renderer.tssrc/client/render/gl/passes/WorldTextPass.tstests/client/graphics/layers/AttackingTroopsOverlay.test.ts
💤 Files with no reviewable changes (1)
- src/client/hud/layers/AttackingTroopsOverlay.ts
github-project-automation
Bot
moved this from Development
to Complete
in OpenFront Release Management
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description:
Replaces the DOM-based
AttackingTroopsOverlaywithAttackingTroopsController, rendering attack troop counts throughWorldTextPassinstead of a separate fixed-position DOM container.Summary
AttackingTroopsControllerpollsattackClusteredPositions()every 200ms and pushes labels to the WebGL view each frame, lerping cluster positions over 250ms for smooth front-line movement (replaces the old CSStransform 0.25stransition).WorldTextPassgainssetAttackTroopLabels()and renders them at a fixed on-screen size (zoom-independent) usingscreenScale / zoom.NamePassso attack callouts aren't hidden behind centered player names.AttackingTroopsOverlay.ts; existing unit tests repointed to the controller's exportedalignClusterOrder.Please complete the following:
Please put your Discord username so you can be contacted if a bug or regression is found:
evan